Skip to content

feat: add FromPropertyValue trait - #72

Open
m4tx wants to merge 1 commit into
mainfrom
property-value-traits
Open

feat: add FromPropertyValue trait#72
m4tx wants to merge 1 commit into
mainfrom
property-value-traits

Conversation

@m4tx

@m4tx m4tx commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread src/standard.rs
}
}

/// Returns the value of the standard `reg` property.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the part I dislike the most. Because of the differences in lifetimes between the fdt and model APIs, I don't think it's possible to avoid code duplication and maintain the ability to use the standard API while dropping the node objects it operates on - something tested by the newly added standard_node_outlives_wrapper test.

These kind of problems might suggest that maybe the GAT refactor from #29 wasn't the best idea ever - we traded the lifetime accuracy and code simplicity for better user ergonomics. Perhaps it's worth rolling back the change, and instead of trying to unify the read and write APIs, just rely on explicit to_read() method on the read APIs that would return a view of Node/Property.

If you have any thoughts on this @qwandor, I'm happy to hear them.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a never a massive fan of the GAT refactor, so I'm happy for it to be rolled back if that helps avoid tying lifetimes together unneccessarily.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. I'll experiment with rolling back the GAT refactor in one of the future PRs and we'll see what happens.

@m4tx
m4tx requested a review from qwandor August 10, 2026 14:38
Comment thread src/standard/cpus.rs Outdated
Comment thread src/standard.rs
}
}

/// Returns the value of the standard `reg` property.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a never a massive fan of the GAT refactor, so I'm happy for it to be rolled back if that helps avoid tying lifetimes together unneccessarily.

Comment thread src/values.rs
Comment thread src/values.rs
Base automatically changed from to-property-value to main August 12, 2026 15:14
@m4tx
m4tx force-pushed the property-value-traits branch from 4b143d6 to 645686b Compare August 20, 2026 13:57
@m4tx
m4tx requested a review from qwandor August 21, 2026 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants